Skip to content

feat: remove rpyc#2094

Open
paul-nechifor wants to merge 3 commits into
mainfrom
paul/feat/remove-rpyc
Open

feat: remove rpyc#2094
paul-nechifor wants to merge 3 commits into
mainfrom
paul/feat/remove-rpyc

Conversation

@paul-nechifor
Copy link
Copy Markdown
Contributor

@paul-nechifor paul-nechifor commented May 15, 2026

Problem

  • We don't want RPyC anymore.

Solution

  • Excise it.

Contributor License Agreement

  • I have read and approved the CLA.

@paul-nechifor paul-nechifor marked this pull request as draft May 15, 2026 04:47
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 15, 2026

Greptile Summary

This PR removes RPyC entirely and replaces it with the project's own LCM-based pub/sub RPC layer (LCMRPC). The per-worker RPyC servers, the rpyc_port registry field, and the associated retry/connect logic are all deleted; a new CoordinatorRPC class exposes the ModuleCoordinator's control surface as named @rpc methods over LCM, and RemoteModuleSource uses that endpoint to list modules and build proxies.

  • CoordinatorRPC handles serve/connect lifecycle; _ensure_no_existing_service prevents double-serving on the same LCM bus.
  • RemoteModuleSource now builds either a full RPCClient (when the module class is importable) or a names-only _RemoteProxy, sharing a single LCMRPC connection with the coordinator.
  • Blueprint gains __getstate__/__setstate__ so its MappingProxyType fields survive the pickle round-trip required by LCMRPC; RunEntry.load drops unknown keys for forward-compatibility with older registry files.

Confidence Score: 5/5

The RPyC removal is clean and well-scoped; the LCM-based replacement follows existing pub/sub patterns and all previously-flagged gaps have been noted in prior threads.

The change is a straightforward excision of one RPC transport and substitution of another already used by the rest of the codebase. New code paths are covered by updated tests, the Blueprint pickle fix is correct and tested, and RunEntry forward-compatibility is handled. The only new findings are cosmetic: a factory method that is defined but never called, and a magic string constant that could use a comment.

No files require special attention beyond what was already flagged in previous review rounds.

Important Files Changed

Filename Overview
dimos/core/coordination/coordinator_rpc.py New file: wraps an LCMRPC instance to serve or connect to the Coordinator endpoint; includes a duplicate-service guard via _ensure_no_existing_service.
dimos/porcelain/remote_module_source.py Rewrites remote module source to use CoordinatorRPC over LCM; introduces _RemoteProxy for unimportable module classes and descriptor caching.
dimos/porcelain/local_module_source.py Simplified: get_module now directly iterates _coordinator._deployed_modules without a lock (unguarded dict traversal under concurrent module deploy/restart).
dimos/core/rpc_client.py RPCClient gains optional shared-rpc constructor (rpc= parameter, _owns_rpc flag) and a remote() factory classmethod; actor_instance=None guard prevents non-@rpc attribute access in remote mode.
dimos/core/coordination/module_coordinator.py Replaces RpycServer with CoordinatorRPC; adds ping/list_modules/load_blueprint_by_name as @rpc-exposed methods; restart_module_by_class_name return type changed to None.
dimos/core/coordination/blueprints.py Blueprint gains getstate/setstate to make MappingProxyType fields picklable for LCMRPC transport; covered by new test_blueprint_pickle_roundtrip.
dimos/porcelain/dimos.py connect() drops host/port params and the 25s timeout cap; adds connect_in_process() for in-process test use; peek_stream rewritten around the new peek_stream @rpc method.
dimos/core/run_registry.py Removes rpyc_port field from RunEntry and get_most_recent_rpyc_port helper; RunEntry.load now drops unknown keys for forward-compat with old registry files.
dimos/robot/unitree/go2/connection.py Adds .lower() normalization for unitree_connection_type and handles stringified-boolean "true" as an alias for MujocoConnection.

Sequence Diagram

sequenceDiagram
    participant CLI as dimos CLI / Dimos.connect()
    participant RMS as RemoteModuleSource
    participant CRPC as CoordinatorRPC
    participant LCM as LCM Bus
    participant MC as ModuleCoordinator
    participant Proxy as RPCClient / _RemoteProxy

    CLI->>RMS: "RemoteModuleSource(timeout=5)"
    RMS->>CRPC: CoordinatorRPC.connect(timeout)
    CRPC->>LCM: call_sync(Coordinator/ping)
    LCM->>MC: ping()
    MC-->>LCM: pong
    LCM-->>CRPC: pong
    CRPC-->>RMS: CoordinatorRPC instance

    CLI->>RMS: list_module_names()
    RMS->>CRPC: call(list_modules)
    CRPC->>LCM: call_sync(Coordinator/list_modules)
    LCM->>MC: list_modules()
    MC-->>LCM: [ModuleDescriptor, ...]
    LCM-->>RMS: descriptors

    CLI->>RMS: get_module(StressTestModule)
    RMS->>RMS: importlib.import_module(qualified_path)
    alt import succeeds
        RMS->>Proxy: "RPCClient(None, cls, rpc=coord.rpc)"
    else import fails
        RMS->>Proxy: _RemoteProxy(coord.rpc, name, rpc_names)
    end
    Proxy-->>CLI: proxy

    CLI->>Proxy: proxy.ping()
    Proxy->>LCM: call_sync(StressTestModule/ping)
    LCM->>MC: ping() on module
    MC-->>LCM: pong
    LCM-->>CLI: pong
Loading

Reviews (4): Last reviewed commit: "Merge branch 'main' into paul/feat/remov..." | Re-trigger Greptile

Comment thread dimos/porcelain/dimos.py Outdated
Comment thread dimos/porcelain/remote_module_source.py
Comment thread dimos/porcelain/remote_module_source.py
@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Comment thread dimos/core/coordination/blueprints.py Outdated
Comment thread dimos/core/coordination/test_module_coordinator.py Outdated
Comment thread dimos/core/module.py
@paul-nechifor paul-nechifor force-pushed the paul/feat/remove-rpyc branch 3 times, most recently from afbb2d4 to 0b95215 Compare May 17, 2026 04:41
@paul-nechifor paul-nechifor marked this pull request as ready for review May 17, 2026 04:44
@paul-nechifor paul-nechifor enabled auto-merge (squash) May 17, 2026 04:44
Comment thread dimos/porcelain/local_module_source.py
Comment thread dimos/porcelain/dimos.py
@paul-nechifor paul-nechifor force-pushed the paul/feat/remove-rpyc branch from c063cac to 93fc89c Compare May 17, 2026 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants